fix(tools): propagate tool registry to subagents by paoloanzn · Pull Request #1711 · sipeed/picoclaw

paoloanzn · 2026-03-17T17:52:07Z

Summary

Fix: SubagentManager is created with an empty ToolRegistry (NewToolRegistry() at subagent.go:48) and SetTools() is never called after the multi-agent refactor in registerSharedTools(). This causes every tool invocation from a subagent to return "tool not found", making subagents completely non-functional.
Root cause: When registerSharedTools() was introduced, the subagentManager.SetTools() call that populated the subagent's tools was dropped. The SetTools() and RegisterTool() methods exist on SubagentManager but are never invoked.
Fix approach: Add ToolRegistry.Clone() to create an independent snapshot of the parent agent's tool registry, then call subagentManager.SetTools(agent.Tools.Clone()) immediately after manager creation but before spawn/spawn_status registration — giving subagents access to file, exec, web, and other tools while preventing recursive subagent spawning.

Changes

File	Change
`pkg/tools/registry.go`	Add `Clone()` method — creates independent shallow copy of a tool registry
`pkg/agent/loop.go`	Add `subagentManager.SetTools(agent.Tools.Clone())` in `registerSharedTools()`
`pkg/tools/registry_test.go`	3 new tests: clone isolation, empty clone, hidden tool state preservation

Why Clone instead of sharing the pointer?

spawn and spawn_status tools are registered on agent.Tools after the subagent manager is created (lines 248-251 of loop.go). Sharing the registry pointer would give subagents access to spawn tools → recursive subagent spawning. Clone() captures a snapshot with only the tools registered at that point.

Test plan

TestToolRegistry_Clone — verifies parent/clone isolation in both directions
TestToolRegistry_Clone_Empty — verifies cloning an empty registry
TestToolRegistry_Clone_PreservesHiddenToolState — verifies hidden tool TTL behavior is preserved
All existing registry tests still pass
pkg/tools and pkg/agent packages build cleanly

Note: pkg/tools test suite has a pre-existing build failure in cron_test.go:229 (SubscribeOutbound undefined on *bus.MessageBus) that exists on main before this change. Our new tests are verified passing on a branch where cron_test.go compiles.

🤖 Generated with Claude Code

SubagentManager was created with an empty ToolRegistry and SetTools() was never called, causing all subagent tool invocations to fail with "tool not found". This was a regression from the multi-agent refactor. Fix: clone the parent agent's tool registry into the subagent manager after creation but before spawn/spawn_status registration — giving subagents access to file, exec, web, and other tools while preventing recursive subagent spawning. - Add ToolRegistry.Clone() for independent shallow copies - Call subagentManager.SetTools(agent.Tools.Clone()) in registerSharedTools - Add tests for Clone isolation, empty clone, and hidden tool state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

CLAassistant · 2026-03-17T17:52:28Z

All committers have signed the CLA.

yinwm

Code Review

This is a critical bugfix that resolves the issue where subagents were completely non-functional due to missing tool propagation. The changes are minimal and well-targeted.

✅ What's Good

Accurate root cause analysis - Correctly identified that SubagentManager was created with an empty ToolRegistry and SetTools() was never called after the multi-agent refactor.
Correct solution - Using Clone() to create a snapshot rather than sharing the pointer effectively prevents recursive subagent spawning while giving subagents access to all necessary tools.
Proper lock usage - Using RLock() instead of Lock() since we're only reading.
Performance optimization - Pre-allocating the map with make(map[string]*ToolEntry, len(r.tools)) avoids multiple allocations.
Good test coverage - Tests verify bidirectional isolation, edge case (empty clone), and hidden tool state preservation.

⚠️ Items to Consider

1. `version` field not copied

clone := &ToolRegistry{
    tools: make(map[string]*ToolEntry, len(r.tools)),
    // version field defaults to 0
}

The version field in ToolRegistry is used for cache invalidation. The clone starts with version=0, which should be fine (the clone is a new independent registry managing its own version). However, it might be worth adding a note in the comment explaining this behavior.

2. Test doesn't verify TTL value is correctly copied

The current test TestToolRegistry_Clone_PreservesHiddenToolState only checks TTL=0 case. Consider adding a test to verify TTL > 0 values are correctly preserved:

func TestToolRegistry_Clone_PreservesTTL(t *testing.T) {
    r := NewToolRegistry()
    tool := newMockTool("hidden_tool", "hidden")
    r.RegisterHidden(tool)
    // Manually set TTL > 0
    if entry, ok := r.tools["hidden_tool"]; ok {
        entry.TTL = 5
    }

    clone := r.Clone()
    if entry, ok := clone.tools["hidden_tool"]; ok {
        if entry.TTL != 5 {
            t.Errorf("expected TTL=5, got %d", entry.TTL)
        }
    }
}

💡 Minor Suggestion (Non-blocking)

The comment could mention that version is reset:

// Clone creates an independent copy of the registry containing the same tool
// entries (shallow copy of each ToolEntry). This is used to give subagents a
// snapshot of the parent agent's tools without sharing the same registry —
// tools registered on the parent after cloning (e.g. spawn, spawn_status)
// will NOT be visible to the clone, preventing recursive subagent spawning.
// The version counter is reset to 0 in the clone as it's a new independent registry.

🎯 Verdict

Aspect	Rating
Correctness	✅ Fix logic is correct
Thread Safety	✅ No concurrency issues
Maintainability	✅ Code is clear
Test Coverage	⚠️ Consider adding TTL value test

LGTM with minor nits — Ready to merge, but recommend adding a test case for TTL value preservation for completeness.

🤖 Generated with Claude Code

yinwm · 2026-03-18T06:34:17Z

plz fix Linter and tests @paoloanzn

- Fix cron_test.go:229 — replace non-existent SubscribeOutbound(ctx) with select on OutboundChan(), matching the MessageBus channel API - Add TestToolRegistry_Clone_PreservesTTLValue per reviewer feedback - Add version reset note to Clone() doc comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

paoloanzn · 2026-03-18T12:03:36Z

@yinwm Thanks for the review! I've pushed a follow-up commit addressing both the CI failures and your review feedback:

Fixes:

Linter + Tests: Fixed pre-existing build error in cron_test.go:229 — replaced non-existent SubscribeOutbound(ctx) with select on OutboundChan(), matching the MessageBus channel API
TTL test: Added TestToolRegistry_Clone_PreservesTTLValue per your review suggestion
Doc comment: Added version reset note to Clone() godoc

All tests pass locally (go test ./pkg/tools/ -count=1 -short ✅).

CI may need approval to re-run since this is a fork PR.

yinwm · 2026-03-18T14:12:14Z

@paoloanzn sorry , there's a conflicts, plz fix

yinwm

LGTM

yinwm · 2026-03-18T16:17:21Z

thanks for this pr

paoloanzn · 2026-03-18T16:46:13Z

thanks for this pr

my pleasure to contribute. best agent system out there rn 😄

Orgmar · 2026-03-19T02:58:38Z

@paoloanzn Great debugging work here. Tracking down the empty ToolRegistry after the multi-agent refactor and using Clone() to snapshot the parent tools while preventing recursive spawning is a well thought out solution. The isolation tests are thorough too.

We have a PicoClaw Dev Group on Discord for contributors to collaborate more directly. Want to join? Send an email to support@sipeed.com with subject [Join PicoClaw Dev Group] paoloanzn and we'll send you the invite link!

paoloanzn · 2026-03-19T17:24:47Z

@paoloanzn Great debugging work here. Tracking down the empty ToolRegistry after the multi-agent refactor and using Clone() to snapshot the parent tools while preventing recursive spawning is a well thought out solution. The isolation tests are thorough too.

We have a PicoClaw Dev Group on Discord for contributors to collaborate more directly. Want to join? Send an email to support@sipeed.com with subject [Join PicoClaw Dev Group] paoloanzn and we'll send you the invite link!

thank you for the invite, i've sent the email!

* fix(tools): propagate tool registry to subagents via Clone SubagentManager was created with an empty ToolRegistry and SetTools() was never called, causing all subagent tool invocations to fail with "tool not found". This was a regression from the multi-agent refactor. Fix: clone the parent agent's tool registry into the subagent manager after creation but before spawn/spawn_status registration — giving subagents access to file, exec, web, and other tools while preventing recursive subagent spawning. - Add ToolRegistry.Clone() for independent shallow copies - Call subagentManager.SetTools(agent.Tools.Clone()) in registerSharedTools - Add tests for Clone isolation, empty clone, and hidden tool state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(tools): fix cron_test build error and add TTL clone test - Fix cron_test.go:229 — replace non-existent SubscribeOutbound(ctx) with select on OutboundChan(), matching the MessageBus channel API - Add TestToolRegistry_Clone_PreservesTTLValue per reviewer feedback - Add version reset note to Clone() doc comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

paoloanzn mentioned this pull request Mar 17, 2026

feat: patch-based custom fork system with 4 patches paoloanzn/neo-picoclaw#1

Merged

5 tasks

github-actions bot mentioned this pull request Mar 18, 2026

🦞 OpenClaw 生态日报 2026-03-18 gsscsd/big_model_radar#56

Open

yinwm reviewed Mar 18, 2026

View reviewed changes

paoloanzn force-pushed the fix/subagent-empty-tools branch from 88b1ea4 to 8851313 Compare March 18, 2026 14:54

Merge branch 'main' into fix/subagent-empty-tools

d0616a0

yinwm approved these changes Mar 18, 2026

View reviewed changes

yinwm merged commit eb86e10 into sipeed:main Mar 18, 2026
4 checks passed

This was referenced Mar 20, 2026

🦞 OpenClaw 生态日报 2026-03-20 gsscsd/big_model_radar#66

Open

🦞 Bản tin hàng ngày hệ sinh thái OpenClaw 2026-03-20 compasify/agents-radar#61

Open

afjcjsbx mentioned this pull request Mar 21, 2026

fix(subagent): ensure spawned subagents receive tools (#1713) #1717

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tools): propagate tool registry to subagents#1711

fix(tools): propagate tool registry to subagents#1711
yinwm merged 3 commits intosipeed:mainfrom
paoloanzn:fix/subagent-empty-tools

paoloanzn commented Mar 17, 2026

Uh oh!

CLAassistant commented Mar 17, 2026 •

edited

Loading

Uh oh!

yinwm left a comment

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

paoloanzn commented Mar 18, 2026

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

yinwm left a comment

Uh oh!

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

paoloanzn commented Mar 18, 2026

Uh oh!

Orgmar commented Mar 19, 2026

Uh oh!

paoloanzn commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

paoloanzn commented Mar 17, 2026

Summary

Changes

Why Clone instead of sharing the pointer?

Test plan

Uh oh!

CLAassistant commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yinwm left a comment

Choose a reason for hiding this comment

Code Review

✅ What's Good

⚠️ Items to Consider

1. version field not copied

2. Test doesn't verify TTL value is correctly copied

💡 Minor Suggestion (Non-blocking)

🎯 Verdict

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

paoloanzn commented Mar 18, 2026

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

yinwm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yinwm commented Mar 18, 2026

Uh oh!

paoloanzn commented Mar 18, 2026

Uh oh!

Orgmar commented Mar 19, 2026

Uh oh!

paoloanzn commented Mar 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Mar 17, 2026 •

edited

Loading

1. `version` field not copied